Repository navigation
Expand float and complex strict mode to allow ints and ints/float (for PEP 484 compatibility). - #5879
Conversation
|
I have started working on this too. |
This is known at compile time so it can be constexpr
|
Here is my implementation and documentation changes. It needs tests but I am going to stop for today. |
|
You will need to remove the constexpr check. I didn't realise it was a newer C++ feature |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
TODO: figure out why it doesn't work in C++11. |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
I have just found that a python float passed to an int argument will raise a TypeError. This is probably something we should resolve here because it effects these tests. m.def("func", [](int){});>>> func.__doc__
func(arg: typing.SupportsInt) -> None
>>> func(5.5)
TypeError
>>> func(numpy.float32(5.5)) # This works fine |
The int type caster allows anything that implements __int__ with explicit exception of the python float. I can't see any reason for this. This modifies the int casting behaviour to accept a float. If the argument is marked as noconvert() it will only accept int.
|
Here is my proposed change. You will need to update the tests. Here are my unittests |
|
I noticed something else to have address. According to the documentation all of int,float,complex support falling back on |
…ere (e.g. UB, or bug in Python beta testing). See also: pybind#5879 (comment)
Same. I looked through everything several times now. Thanks! |
|
FYI — This PR is included in PR #5988 |
As of pybind/pybind11#5879, pybind11 will convert Python `int` to C `float`/`double`. For `std::variant`, pybind11 will attempt to convert arguments in the order of the type. So if `double` occurs earlier in the variant, then `int`/`long` will never be produced. By putting `int`/`long` before `double`, pybind11 will attempt that conversion first and we'll continue to produce our deprecation warning correctly. Fixes matplotlib#31495
As of pybind/pybind11#5879, pybind11 will convert Python `int` to C `float`/`double`. For `std::variant`, pybind11 will attempt to convert arguments in the order of the type. So if `double` occurs earlier in the variant, then `int`/`long` will never be produced. By putting `int`/`long` before `double`, pybind11 will attempt that conversion first and we'll continue to produce our deprecation warning correctly. Fixes #31495
pinning pybind11<3 to dodge pybind11 3.x's overload-resolution change no longer even compiles: scipp 26.8.0's own lib/python/dtype.cpp and bind_data_access.h use pybind11 3.x-only API (dtype::normalized_num, dtype::num_of, py::make_scalar). Root-caused the actual regression instead of just pinning around it: bind_operators.h registers a double overload and an int64_t overload of each scalar operator (__add__, __iadd__, __eq__, ...) in that order. pybind11 3.x loosened float's implicit-conversion check to also accept a Python int (pybind/pybind11#5879), so the double overload - registered first - now wins for a plain int where int64_t used to, silently promoting int-valued scalars/results to float64 throughout scipp's Python layer. That's what actually failed 22 of upstream's own tests here, not a riscv64 codegen issue (confirmed against the official x86_64 PyPI wheel earlier). patches/scipp/26.8.0/0002 reorders registration to int64_t-first, so the exact-type overload wins under either pybind11 generation; nothing else changes. Verified the patch applies cleanly against the real 26.8.0 tag. Drop the now-unused PIP_CONSTRAINT/PIP_BUILD_CONSTRAINT env (no build-constraints.txt is written anymore).
* Port scipp 26.8.0 to riscv64 Multi-dimensional labeled-array library with a pybind11/scikit-build-core C++ core. lib/cmake/scipp-cpm.cmake fetches and builds llnl/units, Boost headers, Eigen3, GoogleTest (test-only) and oneTBB from source for every wheel build, so the port needs no system TBB/Eigen/Boost package; oneTBB 2023.1.0 already builds and publishes for riscv64 in this registry, confirming the toolchain handles it. license-files is an explicit PEP 639 list, so the vendored units/Boost/Eigen3/TBB licences (none of which ship in any published wheel) need widening that list rather than just dropping files at the root. * scipp: Pin version, raise timeout and keep test deps binary-only The riscv64 build of one interpreter takes ~2h55m, so every job hit the 180-minute timeout during the test step. The patched checkout is dirty, so setuptools_scm named the wheel 26.8.1.dev0+g<sha>.d<date>; pin it with SETUPTOOLS_SCM_PRETEND_VERSION_FOR_SCIPP and drop the now-unneeded full history fetch. The test venv also pulled the matplotlib 3.11.2 sdist (and would do the same for pillow and, on cp314t, hypothesis), so extend PIP_ONLY_BINARY to the native test dependencies our registry provides. * scipp: pin pybind11<3 to fix int-scalar dtype corruption cp312/cp314 CI failed 22 of upstream's own tests with DTypeError (add_equals, copy, concat, ... "does not support dtypes 'int64', 'float64'"). Reproduced against the official x86_64 PyPI wheel: those same operations work there, so this isn't a riscv64 codegen issue. Root cause: pyproject.toml's build-system.requires pins pybind11 to no version, so pip resolves the newest at build time. pybind11 3.x changed overload resolution for a plain Python int against overloads registered as (double, int64_t) in that order -- exactly how bind_operators.h registers Variable's scalar arithmetic. 2.x picks int64_t for the int; 3.x now picks double, silently promoting int-valued scalars/results to float64 throughout scipp's Python layer. Confirmed with a minimal pybind11 2.13.6 vs 3.1.0 repro using the same overload order. * build-scipp: fix pybind11 3.x int/double overload dispatch at the source pinning pybind11<3 to dodge pybind11 3.x's overload-resolution change no longer even compiles: scipp 26.8.0's own lib/python/dtype.cpp and bind_data_access.h use pybind11 3.x-only API (dtype::normalized_num, dtype::num_of, py::make_scalar). Root-caused the actual regression instead of just pinning around it: bind_operators.h registers a double overload and an int64_t overload of each scalar operator (__add__, __iadd__, __eq__, ...) in that order. pybind11 3.x loosened float's implicit-conversion check to also accept a Python int (pybind/pybind11#5879), so the double overload - registered first - now wins for a plain int where int64_t used to, silently promoting int-valued scalars/results to float64 throughout scipp's Python layer. That's what actually failed 22 of upstream's own tests here, not a riscv64 codegen issue (confirmed against the official x86_64 PyPI wheel earlier). patches/scipp/26.8.0/0002 reorders registration to int64_t-first, so the exact-type overload wins under either pybind11 generation; nothing else changes. Verified the patch applies cleanly against the real 26.8.0 tag. Drop the now-unused PIP_CONSTRAINT/PIP_BUILD_CONSTRAINT env (no build-constraints.txt is written anymore).
Description
Breaking change to allow Python ints into float when under strict typing (
noconvert), to better match PEP 484 numeric tower rules.This change was also done to complex to allow floats and ints. Now that Python ints can be passed into floats it changes behavior for overload resolution. A method that takes float that is registered before a method that takes an int will now get executed when a Python int is passed in. This overload resolution also affects methods with
std::complex.Resolves #5878
Provides a work around for #5767
NOTE: This PR was reviewed extensively and is approved. However, because of the breaking change, it will be merged only after the next patch release.
Suggested changelog entry:
Breaking change for PEP 484 compatibility: Expand float and complex strict mode to allow ints and ints/float.
📚 Documentation preview 📚: https://pybind11--5879.org.readthedocs.build/